[DirectX] Validate registers are bound to root signature #146785

joaosaffran · 2025-07-02T21:21:44Z

DXC checks if registers are correctly bound to root signature descriptors. This implements the same check.
closes: #126645

github-actions · 2025-07-02T21:24:06Z

✅ With the latest revision this PR passed the C/C++ code formatter.

llvmbot · 2025-07-05T00:40:44Z

@llvm/pr-subscribers-clang

Author: None (joaosaffran)

Changes

DXC checks if registers are correctly bound to root signature descriptors. This implements the same check.

Closes: 126645

Full diff: https://github.com/llvm/llvm-project/pull/146785.diff

7 Files Affected:

(added) clang/test/SemaHLSL/RootSignature-Validation-Fail.hlsl (+35)
(added) clang/test/SemaHLSL/RootSignature-Validation.hlsl (+33)
(modified) llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp (+133-3)
(modified) llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h (+119)
(modified) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll (+2-2)
(modified) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll (+2-2)
(modified) llvm/test/CodeGen/DirectX/llc-pipeline.ll (+1)

diff --git a/clang/test/SemaHLSL/RootSignature-Validation-Fail.hlsl b/clang/test/SemaHLSL/RootSignature-Validation-Fail.hlsl
new file mode 100644
index 0000000000000..b590ed67e7085
--- /dev/null
+++ b/clang/test/SemaHLSL/RootSignature-Validation-Fail.hlsl
@@ -0,0 +1,35 @@
+// RUN: not %clang_dxc -T cs_6_6 -E CSMain %s 2>&1 | FileCheck %s
+
+// CHECK: error: register cbuffer (space=665, register=3) is not defined in Root Signature
+// CHECK: error: register srv (space=0, register=0) is not defined in Root Signature
+// CHECK: error: register uav (space=0, register=4294967295) is not defined in Root Signature
+
+
+#define ROOT_SIGNATURE \
+    "CBV(b3, space=666, visibility=SHADER_VISIBILITY_ALL), " \
+    "DescriptorTable(SRV(t0, space=0, numDescriptors=1), visibility=SHADER_VISIBILITY_VERTEX), " \
+    "DescriptorTable(Sampler(s0, numDescriptors=2), visibility=SHADER_VISIBILITY_ALL), " \
+    "DescriptorTable(UAV(u0, numDescriptors=unbounded), visibility=SHADER_VISIBILITY_ALL)"
+
+cbuffer CB : register(b3, space665) {
+  float a;
+}
+
+StructuredBuffer<int> In : register(t0, space0);
+RWStructuredBuffer<int> Out : register(u0);
+
+RWBuffer<float> UAV : register(u4294967295);
+
+RWBuffer<float> UAV1 : register(u2), UAV2 : register(u4);
+
+RWBuffer<float> UAV3 : register(space0);
+
+
+
+// Compute Shader for UAV testing
+[numthreads(8, 8, 1)]
+[RootSignature(ROOT_SIGNATURE)]
+void CSMain(uint id : SV_GroupID)
+{
+    Out[0] = a + id + In[0] + UAV[0] + UAV1[0] + UAV3[0];
+}
diff --git a/clang/test/SemaHLSL/RootSignature-Validation.hlsl b/clang/test/SemaHLSL/RootSignature-Validation.hlsl
new file mode 100644
index 0000000000000..5a7f5baf00619
--- /dev/null
+++ b/clang/test/SemaHLSL/RootSignature-Validation.hlsl
@@ -0,0 +1,33 @@
+// RUN: %clang_dxc -T cs_6_6 -E CSMain %s 2>&1 
+
+// expected-no-diagnostics
+
+
+#define ROOT_SIGNATURE \
+    "CBV(b3, space=1, visibility=SHADER_VISIBILITY_ALL), " \
+    "DescriptorTable(SRV(t0, space=0, numDescriptors=1), visibility=SHADER_VISIBILITY_ALL), " \
+    "DescriptorTable(Sampler(s0, numDescriptors=2), visibility=SHADER_VISIBILITY_VERTEX), " \
+    "DescriptorTable(UAV(u0, numDescriptors=unbounded), visibility=SHADER_VISIBILITY_ALL)"
+
+cbuffer CB : register(b3, space1) {
+  float a;
+}
+
+StructuredBuffer<int> In : register(t0, space0);
+RWStructuredBuffer<int> Out : register(u0);
+
+RWBuffer<float> UAV : register(u4294967294);
+
+RWBuffer<float> UAV1 : register(u2), UAV2 : register(u4);
+
+RWBuffer<float> UAV3 : register(space0);
+
+
+
+// Compute Shader for UAV testing
+[numthreads(8, 8, 1)]
+[RootSignature(ROOT_SIGNATURE)]
+void CSMain(uint id : SV_GroupID)
+{
+    Out[0] = a + id + In[0] + UAV[0] + UAV1[0] + UAV3[0];
+}
diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp
index 398dcbb8d1737..a52a04323514c 100644
--- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp
+++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "DXILPostOptimizationValidation.h"
+#include "DXILRootSignature.h"
 #include "DXILShaderFlags.h"
 #include "DirectX.h"
 #include "llvm/ADT/SmallString.h"
@@ -84,8 +85,60 @@ static void reportOverlappingBinding(Module &M, DXILResourceMap &DRM) {
   }
 }
 
+static void reportRegNotBound(Module &M, Twine Type,
+                              ResourceInfo::ResourceBinding Binding) {
+  SmallString<128> Message;
+  raw_svector_ostream OS(Message);
+  OS << "register " << Type << " (space=" << Binding.Space
+     << ", register=" << Binding.LowerBound << ")"
+     << " is not defined in Root Signature";
+  M.getContext().diagnose(DiagnosticInfoGeneric(Message));
+}
+
+static dxbc::ShaderVisibility
+tripleToVisibility(llvm::Triple::EnvironmentType ET) {
+  assert((ET == Triple::Pixel || ET == Triple::Vertex ||
+          ET == Triple::Geometry || ET == Triple::Hull ||
+          ET == Triple::Domain || ET == Triple::Mesh ||
+          ET == Triple::Compute) &&
+         "Invalid Triple to shader stage conversion");
+
+  switch (ET) {
+  case Triple::Pixel:
+    return dxbc::ShaderVisibility::Pixel;
+  case Triple::Vertex:
+    return dxbc::ShaderVisibility::Vertex;
+  case Triple::Geometry:
+    return dxbc::ShaderVisibility::Geometry;
+  case Triple::Hull:
+    return dxbc::ShaderVisibility::Hull;
+  case Triple::Domain:
+    return dxbc::ShaderVisibility::Domain;
+  case Triple::Mesh:
+    return dxbc::ShaderVisibility::Mesh;
+  case Triple::Compute:
+    return dxbc::ShaderVisibility::All;
+  default:
+    llvm_unreachable("Invalid triple to shader stage conversion");
+  }
+}
+
+std::optional<mcdxbc::RootSignatureDesc>
+getRootSignature(RootSignatureBindingInfo &RSBI,
+                 dxil::ModuleMetadataInfo &MMI) {
+  if (MMI.EntryPropertyVec.size() == 0)
+    return std::nullopt;
+  std::optional<mcdxbc::RootSignatureDesc> RootSigDesc =
+      RSBI.getDescForFunction(MMI.EntryPropertyVec[0].Entry);
+  if (!RootSigDesc)
+    return std::nullopt;
+  return RootSigDesc;
+}
+
 static void reportErrors(Module &M, DXILResourceMap &DRM,
-                         DXILResourceBindingInfo &DRBI) {
+                         DXILResourceBindingInfo &DRBI,
+                         RootSignatureBindingInfo &RSBI,
+                         dxil::ModuleMetadataInfo &MMI) {
   if (DRM.hasInvalidCounterDirection())
     reportInvalidDirection(M, DRM);
 
@@ -94,14 +147,83 @@ static void reportErrors(Module &M, DXILResourceMap &DRM,
 
   assert(!DRBI.hasImplicitBinding() && "implicit bindings should be handled in "
                                        "DXILResourceImplicitBinding pass");
+
+  if (auto RSD = getRootSignature(RSBI, MMI)) {
+
+    RootSignatureBindingValidation Validation;
+    Validation.addRsBindingInfo(*RSD, tripleToVisibility(MMI.ShaderProfile));
+
+    for (const ResourceInfo &CBuf : DRM.cbuffers()) {
+      ResourceInfo::ResourceBinding Binding = CBuf.getBinding();
+      if (!Validation.checkCRegBinding(Binding))
+        reportRegNotBound(M, "cbuffer", Binding);
+    }
+
+    for (const ResourceInfo &SRV : DRM.srvs()) {
+      ResourceInfo::ResourceBinding Binding = SRV.getBinding();
+      if (!Validation.checkTRegBinding(Binding))
+        reportRegNotBound(M, "srv", Binding);
+    }
+
+    for (const ResourceInfo &UAV : DRM.uavs()) {
+      ResourceInfo::ResourceBinding Binding = UAV.getBinding();
+      if (!Validation.checkURegBinding(Binding))
+        reportRegNotBound(M, "uav", Binding);
+    }
+
+    for (const ResourceInfo &Sampler : DRM.samplers()) {
+      ResourceInfo::ResourceBinding Binding = Sampler.getBinding();
+      if (!Validation.checkSamplerBinding(Binding))
+        reportRegNotBound(M, "sampler", Binding);
+    }
+  }
 }
 } // namespace
 
+void RootSignatureBindingValidation::addRsBindingInfo(
+    mcdxbc::RootSignatureDesc &RSD, dxbc::ShaderVisibility Visibility) {
+  for (size_t I = 0; I < RSD.ParametersContainer.size(); I++) {
+    const auto &[Type, Loc] =
+        RSD.ParametersContainer.getTypeAndLocForParameter(I);
+
+    const auto &Header = RSD.ParametersContainer.getHeader(I);
+    switch (Type) {
+    case llvm::to_underlying(dxbc::RootParameterType::SRV):
+    case llvm::to_underlying(dxbc::RootParameterType::UAV):
+    case llvm::to_underlying(dxbc::RootParameterType::CBV): {
+      dxbc::RTS0::v2::RootDescriptor Desc =
+          RSD.ParametersContainer.getRootDescriptor(Loc);
+
+      if (Header.ShaderVisibility ==
+              llvm::to_underlying(dxbc::ShaderVisibility::All) ||
+          Header.ShaderVisibility == llvm::to_underlying(Visibility))
+        addRange(Desc, Type);
+      break;
+    }
+    case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable): {
+      const mcdxbc::DescriptorTable &Table =
+          RSD.ParametersContainer.getDescriptorTable(Loc);
+
+      for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) {
+        if (Header.ShaderVisibility ==
+                llvm::to_underlying(dxbc::ShaderVisibility::All) ||
+            Header.ShaderVisibility == llvm::to_underlying(Visibility))
+          addRange(Range);
+      }
+      break;
+    }
+    }
+  }
+}
+
 PreservedAnalyses
 DXILPostOptimizationValidation::run(Module &M, ModuleAnalysisManager &MAM) {
   DXILResourceMap &DRM = MAM.getResult<DXILResourceAnalysis>(M);
   DXILResourceBindingInfo &DRBI = MAM.getResult<DXILResourceBindingAnalysis>(M);
-  reportErrors(M, DRM, DRBI);
+  RootSignatureBindingInfo &RSBI = MAM.getResult<RootSignatureAnalysis>(M);
+  ModuleMetadataInfo &MMI = MAM.getResult<DXILMetadataAnalysis>(M);
+
+  reportErrors(M, DRM, DRBI, RSBI, MMI);
   return PreservedAnalyses::all();
 }
 
@@ -113,7 +235,12 @@ class DXILPostOptimizationValidationLegacy : public ModulePass {
         getAnalysis<DXILResourceWrapperPass>().getResourceMap();
     DXILResourceBindingInfo &DRBI =
         getAnalysis<DXILResourceBindingWrapperPass>().getBindingInfo();
-    reportErrors(M, DRM, DRBI);
+    RootSignatureBindingInfo &RSBI =
+        getAnalysis<RootSignatureAnalysisWrapper>().getRSInfo();
+    dxil::ModuleMetadataInfo &MMI =
+        getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata();
+
+    reportErrors(M, DRM, DRBI, RSBI, MMI);
     return false;
   }
   StringRef getPassName() const override {
@@ -125,10 +252,13 @@ class DXILPostOptimizationValidationLegacy : public ModulePass {
   void getAnalysisUsage(llvm::AnalysisUsage &AU) const override {
     AU.addRequired<DXILResourceWrapperPass>();
     AU.addRequired<DXILResourceBindingWrapperPass>();
+    AU.addRequired<RootSignatureAnalysisWrapper>();
+    AU.addRequired<DXILMetadataAnalysisWrapperPass>();
     AU.addPreserved<DXILResourceWrapperPass>();
     AU.addPreserved<DXILResourceBindingWrapperPass>();
     AU.addPreserved<DXILMetadataAnalysisWrapperPass>();
     AU.addPreserved<ShaderFlagsAnalysisWrapper>();
+    AU.addPreserved<RootSignatureAnalysisWrapper>();
   }
 };
 char DXILPostOptimizationValidationLegacy::ID = 0;
diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h
index cb5e624514272..0fa0285425d7e 100644
--- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h
+++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h
@@ -14,10 +14,129 @@
 #ifndef LLVM_LIB_TARGET_DIRECTX_DXILPOSTOPTIMIZATIONVALIDATION_H
 #define LLVM_LIB_TARGET_DIRECTX_DXILPOSTOPTIMIZATIONVALIDATION_H
 
+#include "DXILRootSignature.h"
+#include "llvm/ADT/IntervalMap.h"
+#include "llvm/Analysis/DXILResource.h"
 #include "llvm/IR/PassManager.h"
 
 namespace llvm {
 
+static uint64_t combineUint32ToUint64(uint32_t High, uint32_t Low) {
+  return (static_cast<uint64_t>(High) << 32) | Low;
+}
+
+class RootSignatureBindingValidation {
+  using MapT =
+      llvm::IntervalMap<uint64_t, dxil::ResourceInfo::ResourceBinding,
+                        sizeof(llvm::dxil::ResourceInfo::ResourceBinding),
+                        llvm::IntervalMapInfo<uint64_t>>;
+
+private:
+  MapT::Allocator Allocator;
+  MapT CRegBindingsMap;
+  MapT TRegBindingsMap;
+  MapT URegBindingsMap;
+  MapT SamplersBindingsMap;
+
+  void addRange(const dxbc::RTS0::v2::RootDescriptor &Desc, uint32_t Type) {
+    assert((Type == llvm::to_underlying(dxbc::RootParameterType::CBV) ||
+            Type == llvm::to_underlying(dxbc::RootParameterType::SRV) ||
+            Type == llvm::to_underlying(dxbc::RootParameterType::UAV)) &&
+           "Invalid Type in add Range Method");
+
+    llvm::dxil::ResourceInfo::ResourceBinding Binding;
+    Binding.LowerBound = Desc.ShaderRegister;
+    Binding.Space = Desc.RegisterSpace;
+    Binding.Size = 1;
+
+    uint64_t LowRange =
+        combineUint32ToUint64(Binding.Space, Binding.LowerBound);
+    uint64_t HighRange = combineUint32ToUint64(
+        Binding.Space, Binding.LowerBound + Binding.Size - 1);
+
+    assert(LowRange <= HighRange && "Invalid range configuration");
+
+    switch (Type) {
+
+    case llvm::to_underlying(dxbc::RootParameterType::CBV):
+      CRegBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    case llvm::to_underlying(dxbc::RootParameterType::SRV):
+      TRegBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    case llvm::to_underlying(dxbc::RootParameterType::UAV):
+      URegBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    }
+  }
+
+  void addRange(const dxbc::RTS0::v2::DescriptorRange &Range) {
+
+    llvm::dxil::ResourceInfo::ResourceBinding Binding;
+    Binding.LowerBound = Range.BaseShaderRegister;
+    Binding.Space = Range.RegisterSpace;
+    Binding.Size = Range.NumDescriptors;
+
+    uint64_t LowRange =
+        combineUint32ToUint64(Binding.Space, Binding.LowerBound);
+    uint64_t HighRange = combineUint32ToUint64(
+        Binding.Space, Binding.LowerBound + Binding.Size - 1);
+
+    assert(LowRange <= HighRange && "Invalid range configuration");
+
+    switch (Range.RangeType) {
+    case llvm::to_underlying(dxbc::DescriptorRangeType::CBV):
+      CRegBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    case llvm::to_underlying(dxbc::DescriptorRangeType::SRV):
+      TRegBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    case llvm::to_underlying(dxbc::DescriptorRangeType::UAV):
+      URegBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    case llvm::to_underlying(dxbc::DescriptorRangeType::Sampler):
+      SamplersBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    }
+  }
+
+public:
+  RootSignatureBindingValidation()
+      : Allocator(), CRegBindingsMap(Allocator), TRegBindingsMap(Allocator),
+        URegBindingsMap(Allocator), SamplersBindingsMap(Allocator) {}
+
+  void addRsBindingInfo(mcdxbc::RootSignatureDesc &RSD,
+                        dxbc::ShaderVisibility Visibility);
+
+  bool checkCRegBinding(dxil::ResourceInfo::ResourceBinding Binding) {
+    return CRegBindingsMap.overlaps(
+        combineUint32ToUint64(Binding.Space, Binding.LowerBound),
+        combineUint32ToUint64(Binding.Space,
+                              Binding.LowerBound + Binding.Size - 1));
+  }
+
+  bool checkTRegBinding(dxil::ResourceInfo::ResourceBinding Binding) {
+    return TRegBindingsMap.overlaps(
+        combineUint32ToUint64(Binding.Space, Binding.LowerBound),
+        combineUint32ToUint64(Binding.Space,
+                              Binding.LowerBound + Binding.Size - 1));
+  }
+
+  bool checkURegBinding(dxil::ResourceInfo::ResourceBinding Binding) {
+    return URegBindingsMap.overlaps(
+        combineUint32ToUint64(Binding.Space, Binding.LowerBound),
+        combineUint32ToUint64(Binding.Space,
+                              Binding.LowerBound + Binding.Size - 1));
+  }
+
+  bool checkSamplerBinding(dxil::ResourceInfo::ResourceBinding Binding) {
+    return SamplersBindingsMap.overlaps(
+        combineUint32ToUint64(Binding.Space, Binding.LowerBound),
+        combineUint32ToUint64(Binding.Space,
+                              Binding.LowerBound + Binding.Size - 1));
+  }
+};
+
 class DXILPostOptimizationValidation
     : public PassInfoMixin<DXILPostOptimizationValidation> {
 public:
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll
index 9d89dbdd9107b..053721de1eb1f 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll
@@ -13,7 +13,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 !2 = !{ ptr @main, !3, i32 1 } ; function, root signature
 !3 = !{ !5 } ; list of root signature elements
 !5 = !{ !"DescriptorTable", i32 0, !6, !7 }
-!6 = !{ !"Sampler", i32 0, i32 1, i32 0, i32 -1, i32 1 }
+!6 = !{ !"Sampler", i32 1, i32 1, i32 0, i32 -1, i32 1 }
 !7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 3 }
 
 
@@ -33,7 +33,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 ; DXC-NEXT:             RangesOffset:    44
 ; DXC-NEXT:             Ranges:
 ; DXC-NEXT:               - RangeType:       3
-; DXC-NEXT:                 NumDescriptors:  0
+; DXC-NEXT:                 NumDescriptors:  1
 ; DXC-NEXT:                 BaseShaderRegister: 1
 ; DXC-NEXT:                 RegisterSpace:   0
 ; DXC-NEXT:                 OffsetInDescriptorsFromTableStart: 4294967295
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll
index b516d66180247..8e9b4b43b11a6 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll
@@ -16,7 +16,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 !2 = !{ ptr @main, !3, i32 2 } ; function, root signature
 !3 = !{ !5 } ; list of root signature elements
 !5 = !{ !"DescriptorTable", i32 0, !6, !7 }
-!6 = !{ !"SRV", i32 0, i32 1, i32 0, i32 -1, i32 4 }
+!6 = !{ !"SRV", i32 1, i32 1, i32 0, i32 -1, i32 4 }
 !7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 2 }
 
 ; DXC:  - Name:            RTS0
@@ -35,7 +35,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 ; DXC-NEXT:            RangesOffset:    44
 ; DXC-NEXT:            Ranges:
 ; DXC-NEXT:              - RangeType:       0
-; DXC-NEXT:                NumDescriptors:  0
+; DXC-NEXT:                NumDescriptors:  1
 ; DXC-NEXT:                BaseShaderRegister: 1
 ; DXC-NEXT:                RegisterSpace:   0
 ; DXC-NEXT:                OffsetInDescriptorsFromTableStart: 4294967295
diff --git a/llvm/test/CodeGen/DirectX/llc-pipeline.ll b/llvm/test/CodeGen/DirectX/llc-pipeline.ll
index 2b29fd30a7a56..8d75249dc6ecb 100644
--- a/llvm/test/CodeGen/DirectX/llc-pipeline.ll
+++ b/llvm/test/CodeGen/DirectX/llc-pipeline.ll
@@ -31,6 +31,7 @@
 ; CHECK-NEXT:   DXIL Module Metadata analysis
 ; CHECK-NEXT:   DXIL Shader Flag Analysis
 ; CHECK-NEXT:   DXIL Translate Metadata
+; CHECK-NEXT:   DXIL Root Signature Analysis
 ; CHECK-NEXT:   DXIL Post Optimization Validation
 ; CHECK-NEXT:   DXIL Op Lowering
 ; CHECK-NEXT:   DXIL Prepare Module

llvmbot · 2025-07-05T00:40:45Z

@llvm/pr-subscribers-hlsl

Author: None (joaosaffran)

Changes

DXC checks if registers are correctly bound to root signature descriptors. This implements the same check.

Closes: 126645

Full diff: https://github.com/llvm/llvm-project/pull/146785.diff

7 Files Affected:

(added) clang/test/SemaHLSL/RootSignature-Validation-Fail.hlsl (+35)
(added) clang/test/SemaHLSL/RootSignature-Validation.hlsl (+33)
(modified) llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp (+133-3)
(modified) llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h (+119)
(modified) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll (+2-2)
(modified) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll (+2-2)
(modified) llvm/test/CodeGen/DirectX/llc-pipeline.ll (+1)

diff --git a/clang/test/SemaHLSL/RootSignature-Validation-Fail.hlsl b/clang/test/SemaHLSL/RootSignature-Validation-Fail.hlsl
new file mode 100644
index 0000000000000..b590ed67e7085
--- /dev/null
+++ b/clang/test/SemaHLSL/RootSignature-Validation-Fail.hlsl
@@ -0,0 +1,35 @@
+// RUN: not %clang_dxc -T cs_6_6 -E CSMain %s 2>&1 | FileCheck %s
+
+// CHECK: error: register cbuffer (space=665, register=3) is not defined in Root Signature
+// CHECK: error: register srv (space=0, register=0) is not defined in Root Signature
+// CHECK: error: register uav (space=0, register=4294967295) is not defined in Root Signature
+
+
+#define ROOT_SIGNATURE \
+    "CBV(b3, space=666, visibility=SHADER_VISIBILITY_ALL), " \
+    "DescriptorTable(SRV(t0, space=0, numDescriptors=1), visibility=SHADER_VISIBILITY_VERTEX), " \
+    "DescriptorTable(Sampler(s0, numDescriptors=2), visibility=SHADER_VISIBILITY_ALL), " \
+    "DescriptorTable(UAV(u0, numDescriptors=unbounded), visibility=SHADER_VISIBILITY_ALL)"
+
+cbuffer CB : register(b3, space665) {
+  float a;
+}
+
+StructuredBuffer<int> In : register(t0, space0);
+RWStructuredBuffer<int> Out : register(u0);
+
+RWBuffer<float> UAV : register(u4294967295);
+
+RWBuffer<float> UAV1 : register(u2), UAV2 : register(u4);
+
+RWBuffer<float> UAV3 : register(space0);
+
+
+
+// Compute Shader for UAV testing
+[numthreads(8, 8, 1)]
+[RootSignature(ROOT_SIGNATURE)]
+void CSMain(uint id : SV_GroupID)
+{
+    Out[0] = a + id + In[0] + UAV[0] + UAV1[0] + UAV3[0];
+}
diff --git a/clang/test/SemaHLSL/RootSignature-Validation.hlsl b/clang/test/SemaHLSL/RootSignature-Validation.hlsl
new file mode 100644
index 0000000000000..5a7f5baf00619
--- /dev/null
+++ b/clang/test/SemaHLSL/RootSignature-Validation.hlsl
@@ -0,0 +1,33 @@
+// RUN: %clang_dxc -T cs_6_6 -E CSMain %s 2>&1 
+
+// expected-no-diagnostics
+
+
+#define ROOT_SIGNATURE \
+    "CBV(b3, space=1, visibility=SHADER_VISIBILITY_ALL), " \
+    "DescriptorTable(SRV(t0, space=0, numDescriptors=1), visibility=SHADER_VISIBILITY_ALL), " \
+    "DescriptorTable(Sampler(s0, numDescriptors=2), visibility=SHADER_VISIBILITY_VERTEX), " \
+    "DescriptorTable(UAV(u0, numDescriptors=unbounded), visibility=SHADER_VISIBILITY_ALL)"
+
+cbuffer CB : register(b3, space1) {
+  float a;
+}
+
+StructuredBuffer<int> In : register(t0, space0);
+RWStructuredBuffer<int> Out : register(u0);
+
+RWBuffer<float> UAV : register(u4294967294);
+
+RWBuffer<float> UAV1 : register(u2), UAV2 : register(u4);
+
+RWBuffer<float> UAV3 : register(space0);
+
+
+
+// Compute Shader for UAV testing
+[numthreads(8, 8, 1)]
+[RootSignature(ROOT_SIGNATURE)]
+void CSMain(uint id : SV_GroupID)
+{
+    Out[0] = a + id + In[0] + UAV[0] + UAV1[0] + UAV3[0];
+}
diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp
index 398dcbb8d1737..a52a04323514c 100644
--- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp
+++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "DXILPostOptimizationValidation.h"
+#include "DXILRootSignature.h"
 #include "DXILShaderFlags.h"
 #include "DirectX.h"
 #include "llvm/ADT/SmallString.h"
@@ -84,8 +85,60 @@ static void reportOverlappingBinding(Module &M, DXILResourceMap &DRM) {
   }
 }
 
+static void reportRegNotBound(Module &M, Twine Type,
+                              ResourceInfo::ResourceBinding Binding) {
+  SmallString<128> Message;
+  raw_svector_ostream OS(Message);
+  OS << "register " << Type << " (space=" << Binding.Space
+     << ", register=" << Binding.LowerBound << ")"
+     << " is not defined in Root Signature";
+  M.getContext().diagnose(DiagnosticInfoGeneric(Message));
+}
+
+static dxbc::ShaderVisibility
+tripleToVisibility(llvm::Triple::EnvironmentType ET) {
+  assert((ET == Triple::Pixel || ET == Triple::Vertex ||
+          ET == Triple::Geometry || ET == Triple::Hull ||
+          ET == Triple::Domain || ET == Triple::Mesh ||
+          ET == Triple::Compute) &&
+         "Invalid Triple to shader stage conversion");
+
+  switch (ET) {
+  case Triple::Pixel:
+    return dxbc::ShaderVisibility::Pixel;
+  case Triple::Vertex:
+    return dxbc::ShaderVisibility::Vertex;
+  case Triple::Geometry:
+    return dxbc::ShaderVisibility::Geometry;
+  case Triple::Hull:
+    return dxbc::ShaderVisibility::Hull;
+  case Triple::Domain:
+    return dxbc::ShaderVisibility::Domain;
+  case Triple::Mesh:
+    return dxbc::ShaderVisibility::Mesh;
+  case Triple::Compute:
+    return dxbc::ShaderVisibility::All;
+  default:
+    llvm_unreachable("Invalid triple to shader stage conversion");
+  }
+}
+
+std::optional<mcdxbc::RootSignatureDesc>
+getRootSignature(RootSignatureBindingInfo &RSBI,
+                 dxil::ModuleMetadataInfo &MMI) {
+  if (MMI.EntryPropertyVec.size() == 0)
+    return std::nullopt;
+  std::optional<mcdxbc::RootSignatureDesc> RootSigDesc =
+      RSBI.getDescForFunction(MMI.EntryPropertyVec[0].Entry);
+  if (!RootSigDesc)
+    return std::nullopt;
+  return RootSigDesc;
+}
+
 static void reportErrors(Module &M, DXILResourceMap &DRM,
-                         DXILResourceBindingInfo &DRBI) {
+                         DXILResourceBindingInfo &DRBI,
+                         RootSignatureBindingInfo &RSBI,
+                         dxil::ModuleMetadataInfo &MMI) {
   if (DRM.hasInvalidCounterDirection())
     reportInvalidDirection(M, DRM);
 
@@ -94,14 +147,83 @@ static void reportErrors(Module &M, DXILResourceMap &DRM,
 
   assert(!DRBI.hasImplicitBinding() && "implicit bindings should be handled in "
                                        "DXILResourceImplicitBinding pass");
+
+  if (auto RSD = getRootSignature(RSBI, MMI)) {
+
+    RootSignatureBindingValidation Validation;
+    Validation.addRsBindingInfo(*RSD, tripleToVisibility(MMI.ShaderProfile));
+
+    for (const ResourceInfo &CBuf : DRM.cbuffers()) {
+      ResourceInfo::ResourceBinding Binding = CBuf.getBinding();
+      if (!Validation.checkCRegBinding(Binding))
+        reportRegNotBound(M, "cbuffer", Binding);
+    }
+
+    for (const ResourceInfo &SRV : DRM.srvs()) {
+      ResourceInfo::ResourceBinding Binding = SRV.getBinding();
+      if (!Validation.checkTRegBinding(Binding))
+        reportRegNotBound(M, "srv", Binding);
+    }
+
+    for (const ResourceInfo &UAV : DRM.uavs()) {
+      ResourceInfo::ResourceBinding Binding = UAV.getBinding();
+      if (!Validation.checkURegBinding(Binding))
+        reportRegNotBound(M, "uav", Binding);
+    }
+
+    for (const ResourceInfo &Sampler : DRM.samplers()) {
+      ResourceInfo::ResourceBinding Binding = Sampler.getBinding();
+      if (!Validation.checkSamplerBinding(Binding))
+        reportRegNotBound(M, "sampler", Binding);
+    }
+  }
 }
 } // namespace
 
+void RootSignatureBindingValidation::addRsBindingInfo(
+    mcdxbc::RootSignatureDesc &RSD, dxbc::ShaderVisibility Visibility) {
+  for (size_t I = 0; I < RSD.ParametersContainer.size(); I++) {
+    const auto &[Type, Loc] =
+        RSD.ParametersContainer.getTypeAndLocForParameter(I);
+
+    const auto &Header = RSD.ParametersContainer.getHeader(I);
+    switch (Type) {
+    case llvm::to_underlying(dxbc::RootParameterType::SRV):
+    case llvm::to_underlying(dxbc::RootParameterType::UAV):
+    case llvm::to_underlying(dxbc::RootParameterType::CBV): {
+      dxbc::RTS0::v2::RootDescriptor Desc =
+          RSD.ParametersContainer.getRootDescriptor(Loc);
+
+      if (Header.ShaderVisibility ==
+              llvm::to_underlying(dxbc::ShaderVisibility::All) ||
+          Header.ShaderVisibility == llvm::to_underlying(Visibility))
+        addRange(Desc, Type);
+      break;
+    }
+    case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable): {
+      const mcdxbc::DescriptorTable &Table =
+          RSD.ParametersContainer.getDescriptorTable(Loc);
+
+      for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) {
+        if (Header.ShaderVisibility ==
+                llvm::to_underlying(dxbc::ShaderVisibility::All) ||
+            Header.ShaderVisibility == llvm::to_underlying(Visibility))
+          addRange(Range);
+      }
+      break;
+    }
+    }
+  }
+}
+
 PreservedAnalyses
 DXILPostOptimizationValidation::run(Module &M, ModuleAnalysisManager &MAM) {
   DXILResourceMap &DRM = MAM.getResult<DXILResourceAnalysis>(M);
   DXILResourceBindingInfo &DRBI = MAM.getResult<DXILResourceBindingAnalysis>(M);
-  reportErrors(M, DRM, DRBI);
+  RootSignatureBindingInfo &RSBI = MAM.getResult<RootSignatureAnalysis>(M);
+  ModuleMetadataInfo &MMI = MAM.getResult<DXILMetadataAnalysis>(M);
+
+  reportErrors(M, DRM, DRBI, RSBI, MMI);
   return PreservedAnalyses::all();
 }
 
@@ -113,7 +235,12 @@ class DXILPostOptimizationValidationLegacy : public ModulePass {
         getAnalysis<DXILResourceWrapperPass>().getResourceMap();
     DXILResourceBindingInfo &DRBI =
         getAnalysis<DXILResourceBindingWrapperPass>().getBindingInfo();
-    reportErrors(M, DRM, DRBI);
+    RootSignatureBindingInfo &RSBI =
+        getAnalysis<RootSignatureAnalysisWrapper>().getRSInfo();
+    dxil::ModuleMetadataInfo &MMI =
+        getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata();
+
+    reportErrors(M, DRM, DRBI, RSBI, MMI);
     return false;
   }
   StringRef getPassName() const override {
@@ -125,10 +252,13 @@ class DXILPostOptimizationValidationLegacy : public ModulePass {
   void getAnalysisUsage(llvm::AnalysisUsage &AU) const override {
     AU.addRequired<DXILResourceWrapperPass>();
     AU.addRequired<DXILResourceBindingWrapperPass>();
+    AU.addRequired<RootSignatureAnalysisWrapper>();
+    AU.addRequired<DXILMetadataAnalysisWrapperPass>();
     AU.addPreserved<DXILResourceWrapperPass>();
     AU.addPreserved<DXILResourceBindingWrapperPass>();
     AU.addPreserved<DXILMetadataAnalysisWrapperPass>();
     AU.addPreserved<ShaderFlagsAnalysisWrapper>();
+    AU.addPreserved<RootSignatureAnalysisWrapper>();
   }
 };
 char DXILPostOptimizationValidationLegacy::ID = 0;
diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h
index cb5e624514272..0fa0285425d7e 100644
--- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h
+++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h
@@ -14,10 +14,129 @@
 #ifndef LLVM_LIB_TARGET_DIRECTX_DXILPOSTOPTIMIZATIONVALIDATION_H
 #define LLVM_LIB_TARGET_DIRECTX_DXILPOSTOPTIMIZATIONVALIDATION_H
 
+#include "DXILRootSignature.h"
+#include "llvm/ADT/IntervalMap.h"
+#include "llvm/Analysis/DXILResource.h"
 #include "llvm/IR/PassManager.h"
 
 namespace llvm {
 
+static uint64_t combineUint32ToUint64(uint32_t High, uint32_t Low) {
+  return (static_cast<uint64_t>(High) << 32) | Low;
+}
+
+class RootSignatureBindingValidation {
+  using MapT =
+      llvm::IntervalMap<uint64_t, dxil::ResourceInfo::ResourceBinding,
+                        sizeof(llvm::dxil::ResourceInfo::ResourceBinding),
+                        llvm::IntervalMapInfo<uint64_t>>;
+
+private:
+  MapT::Allocator Allocator;
+  MapT CRegBindingsMap;
+  MapT TRegBindingsMap;
+  MapT URegBindingsMap;
+  MapT SamplersBindingsMap;
+
+  void addRange(const dxbc::RTS0::v2::RootDescriptor &Desc, uint32_t Type) {
+    assert((Type == llvm::to_underlying(dxbc::RootParameterType::CBV) ||
+            Type == llvm::to_underlying(dxbc::RootParameterType::SRV) ||
+            Type == llvm::to_underlying(dxbc::RootParameterType::UAV)) &&
+           "Invalid Type in add Range Method");
+
+    llvm::dxil::ResourceInfo::ResourceBinding Binding;
+    Binding.LowerBound = Desc.ShaderRegister;
+    Binding.Space = Desc.RegisterSpace;
+    Binding.Size = 1;
+
+    uint64_t LowRange =
+        combineUint32ToUint64(Binding.Space, Binding.LowerBound);
+    uint64_t HighRange = combineUint32ToUint64(
+        Binding.Space, Binding.LowerBound + Binding.Size - 1);
+
+    assert(LowRange <= HighRange && "Invalid range configuration");
+
+    switch (Type) {
+
+    case llvm::to_underlying(dxbc::RootParameterType::CBV):
+      CRegBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    case llvm::to_underlying(dxbc::RootParameterType::SRV):
+      TRegBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    case llvm::to_underlying(dxbc::RootParameterType::UAV):
+      URegBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    }
+  }
+
+  void addRange(const dxbc::RTS0::v2::DescriptorRange &Range) {
+
+    llvm::dxil::ResourceInfo::ResourceBinding Binding;
+    Binding.LowerBound = Range.BaseShaderRegister;
+    Binding.Space = Range.RegisterSpace;
+    Binding.Size = Range.NumDescriptors;
+
+    uint64_t LowRange =
+        combineUint32ToUint64(Binding.Space, Binding.LowerBound);
+    uint64_t HighRange = combineUint32ToUint64(
+        Binding.Space, Binding.LowerBound + Binding.Size - 1);
+
+    assert(LowRange <= HighRange && "Invalid range configuration");
+
+    switch (Range.RangeType) {
+    case llvm::to_underlying(dxbc::DescriptorRangeType::CBV):
+      CRegBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    case llvm::to_underlying(dxbc::DescriptorRangeType::SRV):
+      TRegBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    case llvm::to_underlying(dxbc::DescriptorRangeType::UAV):
+      URegBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    case llvm::to_underlying(dxbc::DescriptorRangeType::Sampler):
+      SamplersBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    }
+  }
+
+public:
+  RootSignatureBindingValidation()
+      : Allocator(), CRegBindingsMap(Allocator), TRegBindingsMap(Allocator),
+        URegBindingsMap(Allocator), SamplersBindingsMap(Allocator) {}
+
+  void addRsBindingInfo(mcdxbc::RootSignatureDesc &RSD,
+                        dxbc::ShaderVisibility Visibility);
+
+  bool checkCRegBinding(dxil::ResourceInfo::ResourceBinding Binding) {
+    return CRegBindingsMap.overlaps(
+        combineUint32ToUint64(Binding.Space, Binding.LowerBound),
+        combineUint32ToUint64(Binding.Space,
+                              Binding.LowerBound + Binding.Size - 1));
+  }
+
+  bool checkTRegBinding(dxil::ResourceInfo::ResourceBinding Binding) {
+    return TRegBindingsMap.overlaps(
+        combineUint32ToUint64(Binding.Space, Binding.LowerBound),
+        combineUint32ToUint64(Binding.Space,
+                              Binding.LowerBound + Binding.Size - 1));
+  }
+
+  bool checkURegBinding(dxil::ResourceInfo::ResourceBinding Binding) {
+    return URegBindingsMap.overlaps(
+        combineUint32ToUint64(Binding.Space, Binding.LowerBound),
+        combineUint32ToUint64(Binding.Space,
+                              Binding.LowerBound + Binding.Size - 1));
+  }
+
+  bool checkSamplerBinding(dxil::ResourceInfo::ResourceBinding Binding) {
+    return SamplersBindingsMap.overlaps(
+        combineUint32ToUint64(Binding.Space, Binding.LowerBound),
+        combineUint32ToUint64(Binding.Space,
+                              Binding.LowerBound + Binding.Size - 1));
+  }
+};
+
 class DXILPostOptimizationValidation
     : public PassInfoMixin<DXILPostOptimizationValidation> {
 public:
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll
index 9d89dbdd9107b..053721de1eb1f 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll
@@ -13,7 +13,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 !2 = !{ ptr @main, !3, i32 1 } ; function, root signature
 !3 = !{ !5 } ; list of root signature elements
 !5 = !{ !"DescriptorTable", i32 0, !6, !7 }
-!6 = !{ !"Sampler", i32 0, i32 1, i32 0, i32 -1, i32 1 }
+!6 = !{ !"Sampler", i32 1, i32 1, i32 0, i32 -1, i32 1 }
 !7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 3 }
 
 
@@ -33,7 +33,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 ; DXC-NEXT:             RangesOffset:    44
 ; DXC-NEXT:             Ranges:
 ; DXC-NEXT:               - RangeType:       3
-; DXC-NEXT:                 NumDescriptors:  0
+; DXC-NEXT:                 NumDescriptors:  1
 ; DXC-NEXT:                 BaseShaderRegister: 1
 ; DXC-NEXT:                 RegisterSpace:   0
 ; DXC-NEXT:                 OffsetInDescriptorsFromTableStart: 4294967295
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll
index b516d66180247..8e9b4b43b11a6 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll
@@ -16,7 +16,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 !2 = !{ ptr @main, !3, i32 2 } ; function, root signature
 !3 = !{ !5 } ; list of root signature elements
 !5 = !{ !"DescriptorTable", i32 0, !6, !7 }
-!6 = !{ !"SRV", i32 0, i32 1, i32 0, i32 -1, i32 4 }
+!6 = !{ !"SRV", i32 1, i32 1, i32 0, i32 -1, i32 4 }
 !7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 2 }
 
 ; DXC:  - Name:            RTS0
@@ -35,7 +35,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 ; DXC-NEXT:            RangesOffset:    44
 ; DXC-NEXT:            Ranges:
 ; DXC-NEXT:              - RangeType:       0
-; DXC-NEXT:                NumDescriptors:  0
+; DXC-NEXT:                NumDescriptors:  1
 ; DXC-NEXT:                BaseShaderRegister: 1
 ; DXC-NEXT:                RegisterSpace:   0
 ; DXC-NEXT:                OffsetInDescriptorsFromTableStart: 4294967295
diff --git a/llvm/test/CodeGen/DirectX/llc-pipeline.ll b/llvm/test/CodeGen/DirectX/llc-pipeline.ll
index 2b29fd30a7a56..8d75249dc6ecb 100644
--- a/llvm/test/CodeGen/DirectX/llc-pipeline.ll
+++ b/llvm/test/CodeGen/DirectX/llc-pipeline.ll
@@ -31,6 +31,7 @@
 ; CHECK-NEXT:   DXIL Module Metadata analysis
 ; CHECK-NEXT:   DXIL Shader Flag Analysis
 ; CHECK-NEXT:   DXIL Translate Metadata
+; CHECK-NEXT:   DXIL Root Signature Analysis
 ; CHECK-NEXT:   DXIL Post Optimization Validation
 ; CHECK-NEXT:   DXIL Op Lowering
 ; CHECK-NEXT:   DXIL Prepare Module

llvmbot · 2025-07-05T00:40:46Z

@llvm/pr-subscribers-backend-directx

Author: None (joaosaffran)

Changes

DXC checks if registers are correctly bound to root signature descriptors. This implements the same check.

Closes: 126645

Full diff: https://github.com/llvm/llvm-project/pull/146785.diff

7 Files Affected:

(added) clang/test/SemaHLSL/RootSignature-Validation-Fail.hlsl (+35)
(added) clang/test/SemaHLSL/RootSignature-Validation.hlsl (+33)
(modified) llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp (+133-3)
(modified) llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h (+119)
(modified) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll (+2-2)
(modified) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll (+2-2)
(modified) llvm/test/CodeGen/DirectX/llc-pipeline.ll (+1)

diff --git a/clang/test/SemaHLSL/RootSignature-Validation-Fail.hlsl b/clang/test/SemaHLSL/RootSignature-Validation-Fail.hlsl
new file mode 100644
index 0000000000000..b590ed67e7085
--- /dev/null
+++ b/clang/test/SemaHLSL/RootSignature-Validation-Fail.hlsl
@@ -0,0 +1,35 @@
+// RUN: not %clang_dxc -T cs_6_6 -E CSMain %s 2>&1 | FileCheck %s
+
+// CHECK: error: register cbuffer (space=665, register=3) is not defined in Root Signature
+// CHECK: error: register srv (space=0, register=0) is not defined in Root Signature
+// CHECK: error: register uav (space=0, register=4294967295) is not defined in Root Signature
+
+
+#define ROOT_SIGNATURE \
+    "CBV(b3, space=666, visibility=SHADER_VISIBILITY_ALL), " \
+    "DescriptorTable(SRV(t0, space=0, numDescriptors=1), visibility=SHADER_VISIBILITY_VERTEX), " \
+    "DescriptorTable(Sampler(s0, numDescriptors=2), visibility=SHADER_VISIBILITY_ALL), " \
+    "DescriptorTable(UAV(u0, numDescriptors=unbounded), visibility=SHADER_VISIBILITY_ALL)"
+
+cbuffer CB : register(b3, space665) {
+  float a;
+}
+
+StructuredBuffer<int> In : register(t0, space0);
+RWStructuredBuffer<int> Out : register(u0);
+
+RWBuffer<float> UAV : register(u4294967295);
+
+RWBuffer<float> UAV1 : register(u2), UAV2 : register(u4);
+
+RWBuffer<float> UAV3 : register(space0);
+
+
+
+// Compute Shader for UAV testing
+[numthreads(8, 8, 1)]
+[RootSignature(ROOT_SIGNATURE)]
+void CSMain(uint id : SV_GroupID)
+{
+    Out[0] = a + id + In[0] + UAV[0] + UAV1[0] + UAV3[0];
+}
diff --git a/clang/test/SemaHLSL/RootSignature-Validation.hlsl b/clang/test/SemaHLSL/RootSignature-Validation.hlsl
new file mode 100644
index 0000000000000..5a7f5baf00619
--- /dev/null
+++ b/clang/test/SemaHLSL/RootSignature-Validation.hlsl
@@ -0,0 +1,33 @@
+// RUN: %clang_dxc -T cs_6_6 -E CSMain %s 2>&1 
+
+// expected-no-diagnostics
+
+
+#define ROOT_SIGNATURE \
+    "CBV(b3, space=1, visibility=SHADER_VISIBILITY_ALL), " \
+    "DescriptorTable(SRV(t0, space=0, numDescriptors=1), visibility=SHADER_VISIBILITY_ALL), " \
+    "DescriptorTable(Sampler(s0, numDescriptors=2), visibility=SHADER_VISIBILITY_VERTEX), " \
+    "DescriptorTable(UAV(u0, numDescriptors=unbounded), visibility=SHADER_VISIBILITY_ALL)"
+
+cbuffer CB : register(b3, space1) {
+  float a;
+}
+
+StructuredBuffer<int> In : register(t0, space0);
+RWStructuredBuffer<int> Out : register(u0);
+
+RWBuffer<float> UAV : register(u4294967294);
+
+RWBuffer<float> UAV1 : register(u2), UAV2 : register(u4);
+
+RWBuffer<float> UAV3 : register(space0);
+
+
+
+// Compute Shader for UAV testing
+[numthreads(8, 8, 1)]
+[RootSignature(ROOT_SIGNATURE)]
+void CSMain(uint id : SV_GroupID)
+{
+    Out[0] = a + id + In[0] + UAV[0] + UAV1[0] + UAV3[0];
+}
diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp
index 398dcbb8d1737..a52a04323514c 100644
--- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp
+++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "DXILPostOptimizationValidation.h"
+#include "DXILRootSignature.h"
 #include "DXILShaderFlags.h"
 #include "DirectX.h"
 #include "llvm/ADT/SmallString.h"
@@ -84,8 +85,60 @@ static void reportOverlappingBinding(Module &M, DXILResourceMap &DRM) {
   }
 }
 
+static void reportRegNotBound(Module &M, Twine Type,
+                              ResourceInfo::ResourceBinding Binding) {
+  SmallString<128> Message;
+  raw_svector_ostream OS(Message);
+  OS << "register " << Type << " (space=" << Binding.Space
+     << ", register=" << Binding.LowerBound << ")"
+     << " is not defined in Root Signature";
+  M.getContext().diagnose(DiagnosticInfoGeneric(Message));
+}
+
+static dxbc::ShaderVisibility
+tripleToVisibility(llvm::Triple::EnvironmentType ET) {
+  assert((ET == Triple::Pixel || ET == Triple::Vertex ||
+          ET == Triple::Geometry || ET == Triple::Hull ||
+          ET == Triple::Domain || ET == Triple::Mesh ||
+          ET == Triple::Compute) &&
+         "Invalid Triple to shader stage conversion");
+
+  switch (ET) {
+  case Triple::Pixel:
+    return dxbc::ShaderVisibility::Pixel;
+  case Triple::Vertex:
+    return dxbc::ShaderVisibility::Vertex;
+  case Triple::Geometry:
+    return dxbc::ShaderVisibility::Geometry;
+  case Triple::Hull:
+    return dxbc::ShaderVisibility::Hull;
+  case Triple::Domain:
+    return dxbc::ShaderVisibility::Domain;
+  case Triple::Mesh:
+    return dxbc::ShaderVisibility::Mesh;
+  case Triple::Compute:
+    return dxbc::ShaderVisibility::All;
+  default:
+    llvm_unreachable("Invalid triple to shader stage conversion");
+  }
+}
+
+std::optional<mcdxbc::RootSignatureDesc>
+getRootSignature(RootSignatureBindingInfo &RSBI,
+                 dxil::ModuleMetadataInfo &MMI) {
+  if (MMI.EntryPropertyVec.size() == 0)
+    return std::nullopt;
+  std::optional<mcdxbc::RootSignatureDesc> RootSigDesc =
+      RSBI.getDescForFunction(MMI.EntryPropertyVec[0].Entry);
+  if (!RootSigDesc)
+    return std::nullopt;
+  return RootSigDesc;
+}
+
 static void reportErrors(Module &M, DXILResourceMap &DRM,
-                         DXILResourceBindingInfo &DRBI) {
+                         DXILResourceBindingInfo &DRBI,
+                         RootSignatureBindingInfo &RSBI,
+                         dxil::ModuleMetadataInfo &MMI) {
   if (DRM.hasInvalidCounterDirection())
     reportInvalidDirection(M, DRM);
 
@@ -94,14 +147,83 @@ static void reportErrors(Module &M, DXILResourceMap &DRM,
 
   assert(!DRBI.hasImplicitBinding() && "implicit bindings should be handled in "
                                        "DXILResourceImplicitBinding pass");
+
+  if (auto RSD = getRootSignature(RSBI, MMI)) {
+
+    RootSignatureBindingValidation Validation;
+    Validation.addRsBindingInfo(*RSD, tripleToVisibility(MMI.ShaderProfile));
+
+    for (const ResourceInfo &CBuf : DRM.cbuffers()) {
+      ResourceInfo::ResourceBinding Binding = CBuf.getBinding();
+      if (!Validation.checkCRegBinding(Binding))
+        reportRegNotBound(M, "cbuffer", Binding);
+    }
+
+    for (const ResourceInfo &SRV : DRM.srvs()) {
+      ResourceInfo::ResourceBinding Binding = SRV.getBinding();
+      if (!Validation.checkTRegBinding(Binding))
+        reportRegNotBound(M, "srv", Binding);
+    }
+
+    for (const ResourceInfo &UAV : DRM.uavs()) {
+      ResourceInfo::ResourceBinding Binding = UAV.getBinding();
+      if (!Validation.checkURegBinding(Binding))
+        reportRegNotBound(M, "uav", Binding);
+    }
+
+    for (const ResourceInfo &Sampler : DRM.samplers()) {
+      ResourceInfo::ResourceBinding Binding = Sampler.getBinding();
+      if (!Validation.checkSamplerBinding(Binding))
+        reportRegNotBound(M, "sampler", Binding);
+    }
+  }
 }
 } // namespace
 
+void RootSignatureBindingValidation::addRsBindingInfo(
+    mcdxbc::RootSignatureDesc &RSD, dxbc::ShaderVisibility Visibility) {
+  for (size_t I = 0; I < RSD.ParametersContainer.size(); I++) {
+    const auto &[Type, Loc] =
+        RSD.ParametersContainer.getTypeAndLocForParameter(I);
+
+    const auto &Header = RSD.ParametersContainer.getHeader(I);
+    switch (Type) {
+    case llvm::to_underlying(dxbc::RootParameterType::SRV):
+    case llvm::to_underlying(dxbc::RootParameterType::UAV):
+    case llvm::to_underlying(dxbc::RootParameterType::CBV): {
+      dxbc::RTS0::v2::RootDescriptor Desc =
+          RSD.ParametersContainer.getRootDescriptor(Loc);
+
+      if (Header.ShaderVisibility ==
+              llvm::to_underlying(dxbc::ShaderVisibility::All) ||
+          Header.ShaderVisibility == llvm::to_underlying(Visibility))
+        addRange(Desc, Type);
+      break;
+    }
+    case llvm::to_underlying(dxbc::RootParameterType::DescriptorTable): {
+      const mcdxbc::DescriptorTable &Table =
+          RSD.ParametersContainer.getDescriptorTable(Loc);
+
+      for (const dxbc::RTS0::v2::DescriptorRange &Range : Table.Ranges) {
+        if (Header.ShaderVisibility ==
+                llvm::to_underlying(dxbc::ShaderVisibility::All) ||
+            Header.ShaderVisibility == llvm::to_underlying(Visibility))
+          addRange(Range);
+      }
+      break;
+    }
+    }
+  }
+}
+
 PreservedAnalyses
 DXILPostOptimizationValidation::run(Module &M, ModuleAnalysisManager &MAM) {
   DXILResourceMap &DRM = MAM.getResult<DXILResourceAnalysis>(M);
   DXILResourceBindingInfo &DRBI = MAM.getResult<DXILResourceBindingAnalysis>(M);
-  reportErrors(M, DRM, DRBI);
+  RootSignatureBindingInfo &RSBI = MAM.getResult<RootSignatureAnalysis>(M);
+  ModuleMetadataInfo &MMI = MAM.getResult<DXILMetadataAnalysis>(M);
+
+  reportErrors(M, DRM, DRBI, RSBI, MMI);
   return PreservedAnalyses::all();
 }
 
@@ -113,7 +235,12 @@ class DXILPostOptimizationValidationLegacy : public ModulePass {
         getAnalysis<DXILResourceWrapperPass>().getResourceMap();
     DXILResourceBindingInfo &DRBI =
         getAnalysis<DXILResourceBindingWrapperPass>().getBindingInfo();
-    reportErrors(M, DRM, DRBI);
+    RootSignatureBindingInfo &RSBI =
+        getAnalysis<RootSignatureAnalysisWrapper>().getRSInfo();
+    dxil::ModuleMetadataInfo &MMI =
+        getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata();
+
+    reportErrors(M, DRM, DRBI, RSBI, MMI);
     return false;
   }
   StringRef getPassName() const override {
@@ -125,10 +252,13 @@ class DXILPostOptimizationValidationLegacy : public ModulePass {
   void getAnalysisUsage(llvm::AnalysisUsage &AU) const override {
     AU.addRequired<DXILResourceWrapperPass>();
     AU.addRequired<DXILResourceBindingWrapperPass>();
+    AU.addRequired<RootSignatureAnalysisWrapper>();
+    AU.addRequired<DXILMetadataAnalysisWrapperPass>();
     AU.addPreserved<DXILResourceWrapperPass>();
     AU.addPreserved<DXILResourceBindingWrapperPass>();
     AU.addPreserved<DXILMetadataAnalysisWrapperPass>();
     AU.addPreserved<ShaderFlagsAnalysisWrapper>();
+    AU.addPreserved<RootSignatureAnalysisWrapper>();
   }
 };
 char DXILPostOptimizationValidationLegacy::ID = 0;
diff --git a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h
index cb5e624514272..0fa0285425d7e 100644
--- a/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h
+++ b/llvm/lib/Target/DirectX/DXILPostOptimizationValidation.h
@@ -14,10 +14,129 @@
 #ifndef LLVM_LIB_TARGET_DIRECTX_DXILPOSTOPTIMIZATIONVALIDATION_H
 #define LLVM_LIB_TARGET_DIRECTX_DXILPOSTOPTIMIZATIONVALIDATION_H
 
+#include "DXILRootSignature.h"
+#include "llvm/ADT/IntervalMap.h"
+#include "llvm/Analysis/DXILResource.h"
 #include "llvm/IR/PassManager.h"
 
 namespace llvm {
 
+static uint64_t combineUint32ToUint64(uint32_t High, uint32_t Low) {
+  return (static_cast<uint64_t>(High) << 32) | Low;
+}
+
+class RootSignatureBindingValidation {
+  using MapT =
+      llvm::IntervalMap<uint64_t, dxil::ResourceInfo::ResourceBinding,
+                        sizeof(llvm::dxil::ResourceInfo::ResourceBinding),
+                        llvm::IntervalMapInfo<uint64_t>>;
+
+private:
+  MapT::Allocator Allocator;
+  MapT CRegBindingsMap;
+  MapT TRegBindingsMap;
+  MapT URegBindingsMap;
+  MapT SamplersBindingsMap;
+
+  void addRange(const dxbc::RTS0::v2::RootDescriptor &Desc, uint32_t Type) {
+    assert((Type == llvm::to_underlying(dxbc::RootParameterType::CBV) ||
+            Type == llvm::to_underlying(dxbc::RootParameterType::SRV) ||
+            Type == llvm::to_underlying(dxbc::RootParameterType::UAV)) &&
+           "Invalid Type in add Range Method");
+
+    llvm::dxil::ResourceInfo::ResourceBinding Binding;
+    Binding.LowerBound = Desc.ShaderRegister;
+    Binding.Space = Desc.RegisterSpace;
+    Binding.Size = 1;
+
+    uint64_t LowRange =
+        combineUint32ToUint64(Binding.Space, Binding.LowerBound);
+    uint64_t HighRange = combineUint32ToUint64(
+        Binding.Space, Binding.LowerBound + Binding.Size - 1);
+
+    assert(LowRange <= HighRange && "Invalid range configuration");
+
+    switch (Type) {
+
+    case llvm::to_underlying(dxbc::RootParameterType::CBV):
+      CRegBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    case llvm::to_underlying(dxbc::RootParameterType::SRV):
+      TRegBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    case llvm::to_underlying(dxbc::RootParameterType::UAV):
+      URegBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    }
+  }
+
+  void addRange(const dxbc::RTS0::v2::DescriptorRange &Range) {
+
+    llvm::dxil::ResourceInfo::ResourceBinding Binding;
+    Binding.LowerBound = Range.BaseShaderRegister;
+    Binding.Space = Range.RegisterSpace;
+    Binding.Size = Range.NumDescriptors;
+
+    uint64_t LowRange =
+        combineUint32ToUint64(Binding.Space, Binding.LowerBound);
+    uint64_t HighRange = combineUint32ToUint64(
+        Binding.Space, Binding.LowerBound + Binding.Size - 1);
+
+    assert(LowRange <= HighRange && "Invalid range configuration");
+
+    switch (Range.RangeType) {
+    case llvm::to_underlying(dxbc::DescriptorRangeType::CBV):
+      CRegBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    case llvm::to_underlying(dxbc::DescriptorRangeType::SRV):
+      TRegBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    case llvm::to_underlying(dxbc::DescriptorRangeType::UAV):
+      URegBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    case llvm::to_underlying(dxbc::DescriptorRangeType::Sampler):
+      SamplersBindingsMap.insert(LowRange, HighRange, Binding);
+      break;
+    }
+  }
+
+public:
+  RootSignatureBindingValidation()
+      : Allocator(), CRegBindingsMap(Allocator), TRegBindingsMap(Allocator),
+        URegBindingsMap(Allocator), SamplersBindingsMap(Allocator) {}
+
+  void addRsBindingInfo(mcdxbc::RootSignatureDesc &RSD,
+                        dxbc::ShaderVisibility Visibility);
+
+  bool checkCRegBinding(dxil::ResourceInfo::ResourceBinding Binding) {
+    return CRegBindingsMap.overlaps(
+        combineUint32ToUint64(Binding.Space, Binding.LowerBound),
+        combineUint32ToUint64(Binding.Space,
+                              Binding.LowerBound + Binding.Size - 1));
+  }
+
+  bool checkTRegBinding(dxil::ResourceInfo::ResourceBinding Binding) {
+    return TRegBindingsMap.overlaps(
+        combineUint32ToUint64(Binding.Space, Binding.LowerBound),
+        combineUint32ToUint64(Binding.Space,
+                              Binding.LowerBound + Binding.Size - 1));
+  }
+
+  bool checkURegBinding(dxil::ResourceInfo::ResourceBinding Binding) {
+    return URegBindingsMap.overlaps(
+        combineUint32ToUint64(Binding.Space, Binding.LowerBound),
+        combineUint32ToUint64(Binding.Space,
+                              Binding.LowerBound + Binding.Size - 1));
+  }
+
+  bool checkSamplerBinding(dxil::ResourceInfo::ResourceBinding Binding) {
+    return SamplersBindingsMap.overlaps(
+        combineUint32ToUint64(Binding.Space, Binding.LowerBound),
+        combineUint32ToUint64(Binding.Space,
+                              Binding.LowerBound + Binding.Size - 1));
+  }
+};
+
 class DXILPostOptimizationValidation
     : public PassInfoMixin<DXILPostOptimizationValidation> {
 public:
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll
index 9d89dbdd9107b..053721de1eb1f 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable-AllValidFlagCombinationsV1.ll
@@ -13,7 +13,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 !2 = !{ ptr @main, !3, i32 1 } ; function, root signature
 !3 = !{ !5 } ; list of root signature elements
 !5 = !{ !"DescriptorTable", i32 0, !6, !7 }
-!6 = !{ !"Sampler", i32 0, i32 1, i32 0, i32 -1, i32 1 }
+!6 = !{ !"Sampler", i32 1, i32 1, i32 0, i32 -1, i32 1 }
 !7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 3 }
 
 
@@ -33,7 +33,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 ; DXC-NEXT:             RangesOffset:    44
 ; DXC-NEXT:             Ranges:
 ; DXC-NEXT:               - RangeType:       3
-; DXC-NEXT:                 NumDescriptors:  0
+; DXC-NEXT:                 NumDescriptors:  1
 ; DXC-NEXT:                 BaseShaderRegister: 1
 ; DXC-NEXT:                 RegisterSpace:   0
 ; DXC-NEXT:                 OffsetInDescriptorsFromTableStart: 4294967295
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll
index b516d66180247..8e9b4b43b11a6 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-DescriptorTable.ll
@@ -16,7 +16,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 !2 = !{ ptr @main, !3, i32 2 } ; function, root signature
 !3 = !{ !5 } ; list of root signature elements
 !5 = !{ !"DescriptorTable", i32 0, !6, !7 }
-!6 = !{ !"SRV", i32 0, i32 1, i32 0, i32 -1, i32 4 }
+!6 = !{ !"SRV", i32 1, i32 1, i32 0, i32 -1, i32 4 }
 !7 = !{ !"UAV", i32 5, i32 1, i32 10, i32 5, i32 2 }
 
 ; DXC:  - Name:            RTS0
@@ -35,7 +35,7 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 ; DXC-NEXT:            RangesOffset:    44
 ; DXC-NEXT:            Ranges:
 ; DXC-NEXT:              - RangeType:       0
-; DXC-NEXT:                NumDescriptors:  0
+; DXC-NEXT:                NumDescriptors:  1
 ; DXC-NEXT:                BaseShaderRegister: 1
 ; DXC-NEXT:                RegisterSpace:   0
 ; DXC-NEXT:                OffsetInDescriptorsFromTableStart: 4294967295
diff --git a/llvm/test/CodeGen/DirectX/llc-pipeline.ll b/llvm/test/CodeGen/DirectX/llc-pipeline.ll
index 2b29fd30a7a56..8d75249dc6ecb 100644
--- a/llvm/test/CodeGen/DirectX/llc-pipeline.ll
+++ b/llvm/test/CodeGen/DirectX/llc-pipeline.ll
@@ -31,6 +31,7 @@
 ; CHECK-NEXT:   DXIL Module Metadata analysis
 ; CHECK-NEXT:   DXIL Shader Flag Analysis
 ; CHECK-NEXT:   DXIL Translate Metadata
+; CHECK-NEXT:   DXIL Root Signature Analysis
 ; CHECK-NEXT:   DXIL Post Optimization Validation
 ; CHECK-NEXT:   DXIL Op Lowering
 ; CHECK-NEXT:   DXIL Prepare Module

llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp

inbelic · 2025-07-17T17:59:29Z

llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h

+    uint32_t Start;
+    uint32_t End;
+  };
+  std::unordered_map<dxil::ResourceClass, TypeRange> Ranges;


I still think we can simplify this logic as denote here:

I think we could implement this much simpler with something like:

using TypedBinding = std::pair<Type, ResourceBinding>; llvm::SmallVector<TypedBinding> Bindings;

and then below, instead of Range.find we do a llvm::lower_bound(Bindings, ...) with respect to the type. So that Bindings will be grouped together by type.

Or addBinding just becomes a push_back and then we do llvm::sort by group before returning them in getBindingsOfType

Then getBindingsOfType can take advantage of the sorted nature to return it. I don't think there would be any harm in returning an array ref of TypedBinding either.

I am not sure, that would make it easier, since we would need to make sure the array is sorted as well as using lower_bound to keep track of where each element starts. I am not opposed to this strategy thought, just want to hear @bogner opinion on this first.

The LLVM Programmers manual explicitly states:

We never use containers like unordered_map because they are generally very expensive (each insertion requires a malloc).

(Source: https://llvm.org/docs/ProgrammersManual.html#other-map-like-container-options)

llvm::DenseMap is generally preferred. That said, since dxil::ResourceClass has a valid value range [0,3], maybe we should just declare a 4-element array...

This can be done with an insert-then-query approach, so the sorted vector is a reasonable suggestion. That said, this is the third implementation of pretty much the same algorithm for calculating binding ranges at this point. See my top level comment.

inbelic · 2025-07-17T18:00:34Z

llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp

+    for (const auto &Binding : Bindings) {
+      if (Range.Space == Binding.Space &&
+          Range.LowerBound >= Binding.LowerBound &&
+          Range.UpperBound <= Binding.UpperBound) {


I will forward this comment, just so it doesn't get lost:

Ah yep, sorry.

I guess my original question was about something like:

struct FooData { float data }; ConstantBuffer<FooData> Foo[4] : register(b0); [RootSignature(DescriptorTable(CBV(b0, numDescriptors =2), CBV(b2, numDescriptors = 2))]

Are all registers successfully bound? IIUC, the current logic wouldn't get something like that

DXC doesn't seem to consider this a valid binding scenario: https://hlsl.godbolt.org/z/vYo4cY7vv. So we might not want to make it valid in clang.

inbelic · 2025-07-17T18:03:27Z

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

+  }
+}
+
+ResourceClass ParameterToResourceClass(uint32_t Type) {


The type has been validated before this point right? So we know it is a valid type.

Can we just do ResourceClass(Type) instead?

No because RootSignatureParameter and ResourceClass don't use the same numbers to represent the same resources, for example, the first uses 4 to represent UAV, while the later uses 1.

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h

llvm/lib/Frontend/HLSL/RootSignatureValidations.cpp

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

llvm-beanz · 2025-07-25T14:39:20Z

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

+
+static ResourceClass RangeToResourceClass(uint32_t RangeType) {
+  using namespace dxbc;
+  switch (static_cast<DescriptorRangeType>(RangeType)) {


Should this even be a separate enum? It looks like DescriptorRangeType is identical to ResourceClass.

DescriptorRangeType is supposed to match the binary format representation, so it uses 32 bits and the numbers are supposed to match the ones used in DXC when creating the binary file. Other than that, I think it might be possible to reuse ResourceClass in place of DescriptorRangeType, if we remember to do the correct conversions when reading and writing. My only concern is to create a dependency that might make future code harder to maintain, since we currently extend ResourceClass without the concern of breaking the binary file. Thoughts ?

llvm-beanz · 2025-07-25T14:46:03Z

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

+      continue;
+
+    switch (Type) {
+    case llvm::to_underlying(dxbc::RootParameterType::Constants32Bit): {


Rather than using llvm::to_underlying on all the cases, why not convert the int to enum and handle the potential error outside the switch? That will make the code easier to read.

This was originally a design decision. The decision was made to avoid converting uint_32t to enum in obj2yaml/yaml2obj and when reading and writing root signature to/from the binary format. This simplified the code at the time.

bogner

I think we should probably consolidate some of the duplicate approaches to building ranges out of resource bindings and root signatures. Specifically, I think the logic in DXILResource analysis can be used for this PR, as a small rework of the overlapping range logic in SemaHLSL, and for #148919.

I have a draft PR moving the logic in #150633, which I can clean up if we go with this approach, and a proof of concept in #150634 using this in Sema. WDYT?

bogner · 2025-07-25T15:36:45Z

llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h

+    uint32_t Start;
+    uint32_t End;
+  };
+  std::unordered_map<dxil::ResourceClass, TypeRange> Ranges;


This can be done with an insert-then-query approach, so the sorted vector is a reasonable suggestion. That said, this is the third implementation of pretty much the same algorithm for calculating binding ranges at this point. See my top level comment.

inbelic

I also think some unneeded includes crept in. Eg: <cstdint>

inbelic · 2025-08-05T16:09:41Z

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

+        auto IsBound = Info.isBound(ResList.first, ResBinding.Space, ResRange);
+        if (!IsBound) {
+          reportRegNotBound(M, ResList.first, ResBinding);
+        }


nit:

Suggested change

auto IsBound = Info.isBound(ResList.first, ResBinding.Space, ResRange);

if (!IsBound) {

reportRegNotBound(M, ResList.first, ResBinding);

}

if (!Info.isBound(ResList.first, ResBinding.Space, ResRange)

reportRegNotBound(M, ResList.first, ResBinding);

inbelic · 2025-08-05T16:11:24Z

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

+    bool HasOverlap;
+    hlsl::BindingInfo Info = Builder.calculateBindingInfo(HasOverlap);


We don't seem to use HasOverlap after this. Is that expected? Should we be returning an error if it is true?

There is another PR that takes care of checking if there are overlapping ranges. The original implementation that order made sense, now, I might need to change the PRs orders, will fix that today.

llvm/include/llvm/Frontend/HLSL/HLSLBinding.h

…scriptors-are-bound

inbelic · 2025-08-12T21:07:30Z

llvm/lib/Frontend/HLSL/HLSLBinding.cpp

+  for (BindingRange &R : FreeRanges) {
+    if (B.LowerBound >= R.LowerBound && B.LowerBound < R.UpperBound &&
+        B.UpperBound > R.LowerBound && B.UpperBound <= R.UpperBound)
+      return false;
+  }


I think we can take advantage of FreeRanges being sorted and use llvm::lower_bound for this check

inbelic · 2025-08-12T21:09:18Z

llvm/lib/Frontend/HLSL/HLSLBinding.cpp

+bool BindingInfo::isBound(dxil::ResourceClass RC, uint32_t Space,
+                          BindingRange B) {
+  BindingSpaces &BS = getBindingSpaces(RC);
+  RegisterSpace &RS = BS.getOrInsertSpace(Space);


Is it worth it to add a getSpace that returns an optional if it doesn't exist? It would mean we don't create a bunch of extra RegisteSpaces that need to be traversed on each subsequent getBindingSpaces

@bogner designed this class.
@bogner: thoughts?

inbelic · 2025-08-12T21:15:14Z

llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp

+  // Next checks require that the root signature definition is valid.
+  if (!HasOverlap) {


Does it? We could still report any unbound resources here in the same compile cycle right? Or will the Info not be fully constructed on an error

I am not sure if more error messages here is always beneficial. Since the ranges are already incorrect, the user will need to modify those. So yeah, even I could check that the ranges and report the overlapping ones, I am not sure how correct will that diagnostic be, since the data I am checking is already wrong.

But this is not a blocker, I can produce diagnosis here. I can remove if the team prefers that way.

joaosaffran added 4 commits June 26, 2025 19:34

refactoring

0e8828c

refactoring

2edd215

clean up

242545e

format

3f8dec4

joaosaffran added 2 commits July 2, 2025 21:49

formating

3b1ce3b

fix import issues

f5720af

joaosaffran force-pushed the validation/check-descriptors-are-bound branch from d655039 to d8a91e2 Compare July 4, 2025 19:25

formating

ea54904

joaosaffran changed the title ~~[DirectX][Draft] validate registers are bound to root signature~~ [DirectX] Validate registers are bound to root signature Jul 4, 2025

joaosaffran changed the base branch from users/joaosaffran/146783 to main July 4, 2025 19:39

joaosaffran changed the base branch from main to users/joaosaffran/146783 July 4, 2025 19:40

joaosaffran added 7 commits July 4, 2025 19:44

refactoring

a49aa19

init refactoring

d90676f

adding validation

a04eb9f

clean

5994b8f

implementing

e8b14bf

finish implementing && fix tests

8f40e83

fix issue

28350b2

joaosaffran force-pushed the validation/check-descriptors-are-bound branch from d8a91e2 to 28350b2 Compare July 5, 2025 00:35

joaosaffran added 2 commits July 5, 2025 00:37

sync parent

4fd2e0b

sync parent

e25ee87

joaosaffran marked this pull request as ready for review July 5, 2025 00:40

llvmbot added clang Clang issues not falling into any other category backend:DirectX HLSL HLSL Language Support labels Jul 5, 2025

address comments

881dd36

joaosaffran added 2 commits July 15, 2025 21:28

clean

578a03b

moving code arround

b4a0e16

joaosaffran assigned bogner and inbelic Jul 17, 2025

clean

ef14638

inbelic reviewed Jul 17, 2025

View reviewed changes

addressing comments

662c3a8

joaosaffran requested a review from inbelic July 21, 2025 23:34

llvm-beanz reviewed Jul 25, 2025

View reviewed changes

bogner reviewed Jul 25, 2025

View reviewed changes

Joao Saffran and others added 5 commits July 25, 2025 13:22

address comments

260633c

Merge branch 'main' into validation/check-descriptors-are-bound

d42f156

Merge branch 'main' into validation/check-descriptors-are-bound

9ee3a4b

update code

6db6224

cleanup

04658b8

joaosaffran requested review from llvm-beanz and bogner August 1, 2025 02:55

inbelic reviewed Aug 5, 2025

View reviewed changes

Joao Saffran added 2 commits August 5, 2025 10:45

address comments from inbelic

adf3feb

Merge branch 'validation/overlapping-ranges' into validation/check-de…

fc338b5

…scriptors-are-bound

joaosaffran changed the base branch from main to users/joaosaffran/152229 August 6, 2025 01:10

Joao Saffran added 6 commits August 5, 2025 18:17

clean

f5b5b3e

clean?

03d571a

format

ef51048

formating

21675e6

Merge branch 'validation/overlapping-ranges' into validation/check-de…

47662f0

…scriptors-are-bound

format

6da5fb0

joaosaffran requested a review from inbelic August 12, 2025 17:36

inbelic reviewed Aug 12, 2025

View reviewed changes

addressing inbelic comments

0c72dcf

		bool HasOverlap;
		hlsl::BindingInfo Info = Builder.calculateBindingInfo(HasOverlap);

		// Next checks require that the root signature definition is valid.
		if (!HasOverlap) {

[DirectX] Validate registers are bound to root signature #146785

Are you sure you want to change the base?

[DirectX] Validate registers are bound to root signature #146785

Uh oh!

Conversation

joaosaffran commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 5, 2025

Uh oh!

llvmbot commented Jul 5, 2025

Uh oh!

llvmbot commented Jul 5, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bogner Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bogner left a comment

Choose a reason for hiding this comment

Uh oh!

bogner Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inbelic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joaosaffran commented Jul 2, 2025 •

edited

Loading

github-actions bot commented Jul 2, 2025 •

edited

Loading

bogner Jul 25, 2025 •

edited

Loading

bogner Jul 25, 2025 •

edited

Loading